fix(playground): read compile errors from their metadata, not their frame text - #206
Conversation
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
96351ed to
1f32fac
Compare
PR Preview Deployed (removed)Your changes are live at markojs.com/previews/pr-206. commit 14bde97 |
1f32fac to
abee2eb
Compare
…rame text `CompileError` carries `loc` and `label`, and `CompileErrors` carries one child per diagnostic, but the playground reconstructed all of it by regex over the rendered frame. That lost real information: the caret row keeps the source's tabs, and `( *)` could not match them, so a tab-indented template rendered a build failure with no message, no caret, and a raw `| \t\t^ ...` blob. An aggregate also rendered as a single card. Errors are now normalized from their own properties, and the frame is cut from the workspace source rather than parsed back out of a string, so tabs, CRLF, wide characters and a `|` inside the source stop mattering. `loc` is read in both the nested Marko/Babel shape and Rollup's flat one, so an error from a `.js` or `.css` file in the playground gets the same treatment. The frame parser stays as the fallback for producers that carry no position. `CompileError` does not expose its filename -- the copy in the message is relativized against a cwd the playground does not have -- so the compile call site records it. Adds vitest and covers the metadata path, the tab case, aggregates, a Rollup-shaped error, and both fallbacks. Its config is separate from vite.config.ts because resolving the site's plugin chain roughly triples startup for tests that need none of it; @marko/vite moves to 6.1.9, which no longer crashes when vitest resolves a config without a `test` section.
abee2eb to
14bde97
Compare
1 participant
The playground reconstructed every diagnostic by regex over the rendered code frame, even though the compiler hands it the same information structurally.
CompileErrorcarrieslocandlabel;CompileErrorscarries one child per diagnostic.Parsing the text lost real information. Babel's caret row preserves the source's tabs, and
MARKER_ROW's( *)cannot match a tab, so a tab-indented template rendered a build failure with no message, no caret, and a raw| \t\t^ …blob in the details block with its markdown links unrendered. An aggregate also collapsed into one card.Now the error is read from its own properties, and the frame is cut from the workspace source instead of being parsed back out of a string — so tabs, CRLF, wide characters and a
|inside the source all stop mattering. Tabs are expanded so the caret row and the code row share one grid.locis normalized from both the nested Marko/Babel shape ({start,end}) and Rollup's flat one ({line,column}), so an error from a.jsor.cssfile in the playground gets the same rendering rather than falling back to plain text. The frame parser is kept as the fallback for producers that carry no position at all, such as runtime errors.CompileErrordoes not expose its filename — the copy inside the message is relativized against a cwd the browser does not have (at ../../../../../../tags/index.marko:2:1) — so the compile call site records it. Exposingfilenameupstream would remove that step; worth a small follow-up inmarko.Verified end to end against the real compiler:
\t\t<log/>Line has extra indentation at the beginning, caret under<log/><span>^^^^^^under<span>content=errorsTesting
Adds vitest, wired into CI ahead of the build since these are pure unit tests.
@marko/vitemoves 6.1.8 → 6.1.9 (lockfile only; it is a transitive dep of@marko/run, whose^6.0.3range already allowed it). 6.1.8 threwCannot read properties of undefined (reading 'environment')whenever vitest resolved a config without atestsection, which marko-js/vite#302 fixed.With that in, running against the site's own
vite.config.tsworks — so the separatevitest.config.tsis now a choice rather than a workaround, and it is kept on measured grounds:vitest.config.tsvite.config.tsResolving the site plugin chain roughly triples startup for tests that need none of it.